Skip to content

fix: model list and remove now find files in subdirectories#372

Merged
bigcat88 merged 1 commit into
mainfrom
fix-model-list-subdirs
Mar 12, 2026
Merged

fix: model list and remove now find files in subdirectories#372
bigcat88 merged 1 commit into
mainfrom
fix-model-list-subdirs

Conversation

@bigcat88
Copy link
Copy Markdown
Contributor

Fixes the problem reported in #362 where comfy model list and comfy model remove only looked at files in the top-level model directory. Models stored in subdirectories (like checkpoints/, loras/SD1.5/, etc.) were invisible.

The root cause was list_models() using path.iterdir() which only lists immediate children. Changed it to path.rglob("*") so it finds files at any depth. Also handles missing directories gracefully instead of crashing.

For list_command, added a Type column that shows the subdirectory path (e.g. "checkpoints", "loras/SD1.5") so users can see where each model lives.

For remove, the interactive mode now shows relative paths instead of bare filenames, so users can distinguish between models in different subdirectories. Also added path traversal protection (resolve + is_relative_to check) and switched from exists() to is_file() so passing a directory name doesn't crash with IsADirectoryError.

Tests cover recursive discovery, missing directory handling, directory exclusion, Type column output, path traversal rejection, directory name rejection, subdirectory removal, and root-level removal.

Closes #362

@dosubot dosubot Bot added size:L This PR changes 100-499 lines, ignoring generated files. bug Something isn't working labels Mar 12, 2026
@codecov
Copy link
Copy Markdown

codecov Bot commented Mar 12, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.

@@            Coverage Diff             @@
##             main     #372      +/-   ##
==========================================
+ Coverage   53.36%   54.51%   +1.14%     
==========================================
  Files          32       32              
  Lines        3592     3603      +11     
==========================================
+ Hits         1917     1964      +47     
+ Misses       1675     1639      -36     
Files with missing lines Coverage Δ
comfy_cli/command/models/models.py 56.56% <100.00%> (+16.56%) ⬆️

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@bigcat88 bigcat88 force-pushed the fix-model-list-subdirs branch from 617f3b0 to 7eab073 Compare March 12, 2026 13:56
@bigcat88 bigcat88 merged commit ccac968 into main Mar 12, 2026
14 checks passed
@bigcat88 bigcat88 deleted the fix-model-list-subdirs branch March 12, 2026 14:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working size:L This PR changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant